Fix #20475 improve labels in Deck dialogs#20492
Fix #20475 improve labels in Deck dialogs#20492UdaybhanRaj-Dev wants to merge 1 commit intoankidroid:mainfrom
Conversation
|
Important Maintainers: This PR contains Strings changes
|
|
Hi. Just a couple of suggestions:
|
Thank you for the suggestions. I have shortened the commit message and updated the PR description with the checklist and testing details. |
2caf48f to
b4ac4fd
Compare
|
Hi @UdaybhanRaj-Dev, using semantic commits is a good approach. Try to use that in future PRs. |
Thank you @Ayush-Patel-56 for the suggestion. I will follow semantic commit conventions in future PRs. |
|
|
||
|
|
||
| <com.google.android.material.textfield.TextInputLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| <com.google.android.material.textfield.TextInputLayout |
| // we need the fully-qualified name for subdecks | ||
|
|
||
| val maybeDeckName = fullyQualifyDeckName(dialogText = text) | ||
| // if the name is empty, it seems distracting to show an error |
There was a problem hiding this comment.
May I know why you removed this comment ?
| positiveButton(positiveButtonTextRes) { | ||
| onPositiveButtonClicked() | ||
| } | ||
| positiveButton(positiveButtonTextRes) { onPositiveButtonClicked() } |
| } else { | ||
| null | ||
| } | ||
| if (text.containsNumberLargerThanNine()) context.getString(R.string.create_deck_numeric_hint) else null |
| .Builder(context) | ||
| .show { | ||
| title(title) | ||
| // Resource ID for the dialog's positive action button text. |
There was a problem hiding this comment.
copy/paste: May I know why you removed this comment ?
| // --- Dynamic Hint for 3 different dialogs --- | ||
| val inputLayout = dialog.getInputTextLayout() | ||
| when (deckDialogType) { | ||
| DeckDialogType.DECK -> inputLayout.hint = context.getString(R.string.deck_name) |
There was a problem hiding this comment.
the hint is set inside the .input { } callback, which runs on text updates
Was this intentionally done per-keystroke, or can we set the hint once when the dialog is created ?
|
|
||
| <string name="deck_name">Deck name</string> | ||
| <string name="sub_deck_name">Subdeck name</string> | ||
| <string name="new_deck_name">New Deck name</string> |
There was a problem hiding this comment.
It looks inconsistent with sentence case
| DeckDialogType.RENAME_DECK -> inputLayout.hint = context.getString(R.string.new_deck_name) | ||
| DeckDialogType.FILTERED_DECK -> inputLayout.hint = context.getString(R.string.deck_name) | ||
| } | ||
| // --- End Dynamic Hint --- |
| }.input(prefill = initialDeckName, displayKeyboard = true, waitForPositiveButton = false) { dialog, text -> | ||
|
|
||
| // defining the action of done button in ImeKeyBoard and enter button in physical keyBoard | ||
| // --- Dynamic Hint for 3 different dialogs --- |
There was a problem hiding this comment.
remove this
instead explain in comment properly
|
Hi. I think it's better if you changed the strings:
The reason is to unify this and similar dialogs, shorten the text, and because the titles already convey the action required. |
|
Hi @sanjaysargam , Thank you for the detailed review. I understand that I introduced some unnecessary changes beyond the scope of the issue. I will clean up the PR by removing unrelated changes, extra comments, and simplifying the implementation. I will also move or remove logic that does not need to run on every input update and ensure the code follows existing patterns. I’ll update the PR shortly. Thanks again for your feedback! |
|
Hi @ZornHadNoChoice, Thank you for the suggestion. That makes sense — using shorter and consistent labels improves clarity across dialogs. I will update the strings as suggested:
and align them with similar dialogs. I’ll push the changes shortly. Thanks for the guidance! |
|
Hi, closing this as-per: https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md#new-contributors The replies look LLM generated, and this PR fails my "You have performed a self-review of your own code" check - there's a number of comments which seem to have been removed for no reason |
|
Hi, |
Purpose / Description
This PR improves the input field labels used in the following dialogs:
Previously the dialog used a generic text input field without a clear label, which could make it unclear what users should enter.
This change adds contextual hints depending on the dialog type to improve clarity and user experience.
Fixes
Fixes #20475
Approach
The dialogs use a shared reusable layout
dialog_generic_text_input.xml.Instead of creating multiple layouts, the hint for the
TextInputLayoutis set dynamically inCreateDeckDialog.ktdepending on theDeckDialogType.The following hints are applied:
New string resources added:
deck_namesub_deck_namenew_deck_nameThis keeps the UI layout reusable while providing clearer input labels for each dialog.
How Has This Been Tested?
Tested locally by running the app and verifying the dialogs.
Steps:
Test environment:
Tested using a physical Android device and emulator
The existing validation and dialog behavior remain unchanged.
Learning (optional, can help others)
While working on this issue, I explored how reusable dialog layouts are implemented in the project and how
TextInputLayouthints can be updated dynamically depending on dialog context.Screenshots
Checklist
(https://play.google.com/store/apps/details?id=com.google.android.apps.accessibility.auditor)